-
Notifications
You must be signed in to change notification settings - Fork 205
test: Updates all test workflows to use advanced_cluster TPF implementation #3580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: CLOUDP-320243-dev-2.0.0
Are you sure you want to change the base?
Conversation
failing check is unrelated to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes test suite failures by addressing HCL configuration generation issues and test compatibility problems. The changes focus on correcting Terraform configuration syntax and ensuring proper test execution.
- Updates HCL configuration generation to use TupleVal instead of ListVal for better type flexibility
- Fixes Terraform configuration syntax in test files to use proper assignment operators
- Adds version compatibility checks for migration tests
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
internal/testutil/acc/config_cluster.go | Refactors variable names, fixes indentation, and changes from ListVal to TupleVal for replication specs |
internal/serviceapi/searchdeploymentapi/resource_test.go | Updates cluster configuration syntax from block to assignment format |
internal/service/clusteroutagesimulation/resource_test.go | Adds blank line for import formatting |
internal/service/clusteroutagesimulation/resource_migration_test.go | Adds version compatibility checks for migration tests |
.github/workflows/code-health.yml | Changes reduced_tests flag to true for workflow optimization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did the advanced_cluster acceptance tests go? I cannot find them in the tree: internal/service/advancedcluster/resource_advanced_cluster_test.go found them in resource_test in tpf package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -14,7 +14,7 @@ on: | |||
type: string | |||
required: false | |||
test_group: | |||
description: 'Test group to run, e.g. advanced_cluster, empty for all' | |||
description: 'Test group to run, e.g. advanced_cluster_tpf, empty for all' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about renaming all occurences in this file from advanced_cluster_tpf to advanced_cluster as that's the only one now?
@@ -436,7 +431,7 @@ jobs: | |||
MONGODB_ATLAS_LAST_1X_VERSION: ${{ inputs.mongodb_atlas_last_1x_version }} | |||
ACCTEST_REGEX_RUN: '^TestMig' | |||
ACCTEST_PACKAGES: | | |||
./internal/service/advancedcluster | |||
./internal/service/advancedclustertpf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need advanced_cluster_tpf_mig_from_sdkv2 tests? Maybe we can remove it, and remove logic associated with env.var MONGODB_ATLAS_TEST_SDKV2_TO_TPF from the code.
} | ||
rcMap["auto_scaling"] = cty.ObjectVal(map[string]cty.Value{ | ||
"disk_gb_enabled": cty.BoolVal(asDisk.GetEnabled()), | ||
}) | ||
} | ||
|
||
nodeSpec := rc.GetElectableSpecs() | ||
es := rc.GetElectableSpecs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these changes related with test workflow changes?
@@ -91,4 +91,4 @@ jobs: | |||
secrets: inherit | |||
uses: ./.github/workflows/acceptance-tests.yml | |||
with: | |||
reduced_tests: false | |||
reduced_tests: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you clarify here the why?
@@ -14,7 +14,7 @@ on: | |||
type: string | |||
required: false | |||
test_group: | |||
description: 'Test group to run, e.g. advanced_cluster, empty for all' | |||
description: 'Test group to run, e.g. advanced_cluster_tpf, empty for all' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious about the context: why do we still need to specify the _tpf?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i made a similar comment, we can remove tpf now that it's the only test group
Description
Test suite run: https://github.com/mongodb/terraform-provider-mongodbatlas/actions/runs/16890016640
This PR also addresses all remaining code changes mentioned in CLOUDP-336437
Link to any related issue(s): CLOUDP-336437
Type of change:
Required Checklist:
Further comments